Define the new optional properties "centerToReset" and "centerToZoom" #7
Conversation
|
@dtrucs thanks for this PR, I'll ask for a few changes! |
| * "zoom" and "center" properties are useful when the map is mounted in a particular position | ||
| * and you need this button to reset the view to another position. | ||
| */ | ||
| zoomToReset?: number; |
There was a problem hiding this comment.
Can you document these with a comment per property, like title and icon?
| * "zoom" and "center" properties are useful when the map is mounted in a particular position | ||
| * and you need this button to reset the view to another position. | ||
| */ | ||
| zoomToReset?: number; |
There was a problem hiding this comment.
What do you think of naming these defaultZoom and defaultCenter?
There was a problem hiding this comment.
At the start of this contribution, I also thought of these names.
But once I saw the whole thing, I wonder if it wouldn't be ambiguous with the mapContainer's center/zoom definition.
<MapContainer zoom={defaultMapZoom} center={defaultMapCenter}>
<ResetViewControl
defaultZoom={defaultZoom}
defaultCenter={defaultCenter}
/>
</MapContainer>Maybe the couple targetZoom / targetCenter?
But if you think defaultZoom / defaultCenter is better, i'm totally okay to change it!
There was a problem hiding this comment.
Any though about this @valentinogagliardi?
__tests__/ResetViewControl.spec.tsx
Outdated
|
|
||
| describe("ResetViewControl", () => { | ||
| const mockHandleViewReset = jest.fn(); | ||
| let mapInstance = null as Map | null | ||
| const defaultMapCenter = [-96.8716348, 32.8205866] as LatLngExpression; |
There was a problem hiding this comment.
let's use new LatLng
dtrucs
force-pushed
the
Define-centerToReset-and-centerToZoom-properties
branch
from
88190ca to
8cfb1bc
Compare
Thanks for this plugin!
This PR is a small enhancement to explicitly set the view to reset on a center/zoom other than those mounted by the map.